fix Issue 18905 - [Reg 2.079] C++ classes can no longer be used with …#8304
fix Issue 18905 - [Reg 2.079] C++ classes can no longer be used with …#8304dlang-bot merged 1 commit intodlang:masterfrom
Conversation
|
Thanks for your pull request, @WalterBright! Bugzilla references
|
|
The code between the new |
| @@ -0,0 +1,3 @@ | |||
| // https://issues.dlang.org/show_bug.cgi?id=18905 | |||
There was a problem hiding this comment.
Don't you need -betterC compiler flags here? Also, you might want to add this to the existing betterc.d test file.
| return; | ||
| } | ||
|
|
||
| const bool gentypeinfo = global.params.useTypeInfo && Type.dtypeinfo; |
There was a problem hiding this comment.
If you merge dlang/druntime#2184, then you'll only need to check Type.dtypeinfo because the runtime will no longer import the TypeInfo classes when compiling with -betterC. There are other places in the DMD source code that can be simplified due to that PR.
There was a problem hiding this comment.
I'm not so sure about that. 2184 seems to be more concerned with the nothrow issue, which is unrelated to this PR's fix. It isn't ready yet, either, and this PR fix works and is pretty simple.
There was a problem hiding this comment.
2184 is actually more concerned with preventing the import of code which is not relevant to a -betterC build. The nothrow addition was added because -betterC code should be nothrow at all times. If you want me to separate the two into different PRs, I can. Just let me know. And, it is all green and ready to go.
There was a problem hiding this comment.
I'm not too happy about the nothrow business, as I mentioned. But we should discuss that elsewhere, not here. I also do not believe that 2184 will resolve this issue, as note it took two different tests to get it through the test suite, not just checking typeinfo.
There was a problem hiding this comment.
Understood. Please see dlang/druntime#2194. It prevents unnecessary imports in -betterC, and removes the nothrow issue.
I don't think you need to worry about that anymore. GitHub now has a "Hide whitespace changes" option under the "Diff Settings" on the review tab to aid with that:
...but it's no big deal either way. |
|
I know about the no whitespace view, but I wanted to yank all that code out into a separate function anyway, and that view won't help with that. If I yank it out now, then nobody will want to review this because it's multiple screens of diffs. |
|
@CyberShadow the following error appears in the CyberShadow/DAutoTest: but scanning the voluminous log, I can't see what the actual error is. Please advise! |
|
It looks like a problem with Dub. It seems to have exited with a non-zero exit code without printing an error message. Try rebasing and force-pushing to force a rebuild. |
Already tried it. Same result. |
|
It still looks like an intermittent failure. See http://dtest.dlang.io/history/master - a build of the master branch recently failed with a similar problem, but got fixed by itself. |
|
Also the last stable branch build is failing with the same issue: |
|
It looks like |
MartinNowak
left a comment
MartinNowak
left a comment
There was a problem hiding this comment.
Should have targeted stable.
| } | ||
| } | ||
| flags |= ClassFlags.noPointers; | ||
| L2: |
There was a problem hiding this comment.
Please avoid seemingly unrelated refactorings in bugfixes or mention why they were necessary.
6 participants
…-betterC